HDDS-15486 Remove excessive debug logging in BlockOutputStream and Bu…#10439
HDDS-15486 Remove excessive debug logging in BlockOutputStream and Bu…#10439yandrey321 wants to merge 2 commits into
Conversation
| releaser.start(); | ||
| allocator.join(); | ||
| assertEquals(toRelease, allocated.get()); | ||
| assertTrue(logCapturer.getOutput().contains("Allocation needs to wait the pool is at capacity")); |
There was a problem hiding this comment.
I think we are losing testing coverage if you remove this line.
We might be able to do the following above
assertTrue(pool.allocated.size() == pool.capacity)
allocated.set(pool.allocateBuffer(0));
However I am not completely sure.
To be safe, we can keep this type of verification. Other removals over LOG_DEBUG look ok.
There was a problem hiding this comment.
I'd avoid validation through log messages and prefer to do it via actual state check
There was a problem hiding this comment.
there is a check if pool is full at the beginning of the function:
assertFull(pool);
There was a problem hiding this comment.
Good point. Yes assertFull(pool) does state checks.
I am wondering if other people can also take a look at this before we merge. cc @adoroszlai
| // As the pool is full, allocation will need to wait until a buffer is released. | ||
| assertFull(pool); | ||
|
|
||
| assertEquals(buffers.size(), pool.getAllocatedBuffers().size()); |
There was a problem hiding this comment.
Can we consolidate this line into assertFull?
2 participants
Remove excessive debug logging in BlockOutputStream and BufferPool
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15486
How was this patch tested?
Unit tests